Skip to content

fix(qt): keep PoSe score visible when hiding banned masternodes#7298

Open
thepastaclaw wants to merge 1 commit into
dashpay:developfrom
thepastaclaw:fix/masternode-hide-banned-keep-pose
Open

fix(qt): keep PoSe score visible when hiding banned masternodes#7298
thepastaclaw wants to merge 1 commit into
dashpay:developfrom
thepastaclaw:fix/masternode-hide-banned-keep-pose

Conversation

@thepastaclaw
Copy link
Copy Markdown

PR description

Summary

  • Keep the Masternodes tab PoSe Score column visible when "Hide banned" is
    checked.
  • Continue filtering banned masternodes via the existing proxy filter.

Closes #7286.

Validation

  • git diff --check
  • Pre-PR review gate: ship
  • Not run: full build / GUI smoke test; no build directory was available in
    this worktree.

@thepastaclaw
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 95c1b233-8322-4945-8fc6-b30ee10d4795

📥 Commits

Reviewing files that changed from the base of the PR and between 1d9e004 and 536f388.

📒 Files selected for processing (1)
  • src/qt/masternodelist.cpp
💤 Files with no reviewable changes (1)
  • src/qt/masternodelist.cpp

Walkthrough

The "Hide banned" checkbox handler in the masternode list was modified to remove the logic that hid the POSE table column. The handler now focuses solely on filtering banned rows through the proxy model by calling setHideBanned(...), invalidating the filter, recalculating row counts, and persisting the user preference.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: keeping the PoSe Score column visible while hiding banned masternodes, which matches the core objective of the changeset.
Description check ✅ Passed The description is directly related to the changeset, explaining the intent to keep the PoSe Score column visible when hiding banned masternodes and referencing the closed issue.
Linked Issues check ✅ Passed The code changes directly address the linked issue #7286 by modifying the checkbox handler to filter banned masternodes without hiding the PoSe Score column.
Out of Scope Changes check ✅ Passed All changes in the pull request are scoped to the specific objective of keeping the PoSe Score column visible while filtering banned masternodes; no extraneous modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thepastaclaw
Copy link
Copy Markdown
Author

thepastaclaw commented Apr 29, 2026

✅ Review complete (commit 1d9e004)

Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Minimal one-line fix that correctly decouples the POSE column visibility from the 'Hide banned' filter. The previous behavior was asymmetric (toggling on hid the column but toggling off never restored it). The proxy filter still excludes banned rows. Both reviewers converged on a single test-coverage suggestion; no blocking issues.

Reviewed commit: 1d9e004

💬 1 nitpick(s)

1 additional finding

💬 nitpick: No Qt test covers the Hide Banned / PoSe column behavior

src/qt/masternodelist.cpp (lines 291-300)

The fix lives entirely in MasternodeList::on_checkBoxHideBanned_stateChanged, but no test under src/qt/test/ exercises MasternodeList. The Qt test harness already exists in src/qt/test/test_main.cpp, so a focused test that toggles checkBoxHideBanned and asserts both the filtered row count and that tableViewMasternodes->isColumnHidden(MasternodeModel::POSE) remains false would prevent the same regression from reappearing. Not blocking — the fix is small and correct — but the original bug slipped through precisely because there was no automated coverage of this surface.

Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Single-line Qt fix that correctly stops the 'Hide banned' toggle from also hiding the PoSe Score column. Both reviewers converge on the same nitpick — no Qt test coverage for this widget interaction. No blocking issues.

Reviewed commit: 1d9e004

Fresh dispatcher run for this queue item. A same-SHA review already existed, so this records the fresh verification without duplicating inline threads.

Code Review

Reviewed commit: 1d9e004

💬 1 nitpick(s)

@PastaPastaPasta PastaPastaPasta marked this pull request as ready for review May 19, 2026 14:41
@PastaPastaPasta PastaPastaPasta requested a review from knst May 19, 2026 14:41
Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PastaPastaPasta PastaPastaPasta requested a review from UdjinM6 May 19, 2026 14:42
Copy link
Copy Markdown

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 1d9e004

@thepastaclaw thepastaclaw force-pushed the fix/masternode-hide-banned-keep-pose branch from 1d9e004 to 536f388 Compare May 20, 2026 15:36
Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

PR #7298 is a minimal one-line Qt fix to MasternodeList::on_checkBoxHideBanned_stateChanged that decouples the PoSe Score column visibility from the 'Hide banned' filter. Banned-row filtering still happens via the proxy model (setHideBanned + forceInvalidateFilter), while the PoSe column remains visible — which is desirable since non-zero PoSe scores warn of pre-ban state. The latest delta from 1d9e004 to 536f388 contains only unrelated develop-branch merges; src/qt/masternodelist.cpp is unchanged between the prior review and current head, so the sole carried-forward nitpick (no Qt test covering this interaction) remains valid. No blocking issues, no new latest-delta findings.

💬 1 nitpick(s)

1 additional finding(s) omitted (not in diff).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[UI] - On the Masternodes Tab, the check box 'Hide Banned'...

3 participants